New locking for frame status#1627
Merged
Merged
Conversation
DawidWesierski4
requested review from
Sakoram,
awilczyns,
moleksy,
soopel and
staszczuk
as code owners
DawidWesierski4
force-pushed
the
new_locking_for_frame_status
branch
4 times, most recently
from
699c49c to
dedc1d4
Compare
DawidWesierski4
force-pushed
the
new_locking_for_frame_status
branch
from
dedc1d4 to
873f59e
Compare
Collaborator
|
We are mixing GCC buildins with C11 _Atomic (from stdatomic.h) in this project. |
Sakoram
reviewed
Jul 14, 2026
Collaborator
|
please write somewhere in docs, possibly in design.md our assumption of thread safety because it will work only if frame_get and frame_put are called from the same thread and are synchronous. Also our assumption is that all functions called from lower layers like frame_ready are synchronous. |
DawidWesierski4
force-pushed
the
new_locking_for_frame_status
branch
from
873f59e to
132e360
Compare
Sakoram
approved these changes
Jul 15, 2026
Sakoram
left a comment
Sakoram
left a comment
Collaborator
There was a problem hiding this comment.
Great job.
This looks much better then current mutexes
DawidWesierski4
force-pushed
the
new_locking_for_frame_status
branch
from
132e360 to
75d256c
Compare
Remove the pthread_mutex_t from all ST2110 media pipeline contexts (ST20, ST22, ST30, and ST40) and replace the internal framebuffer state tracking with a lock-free design using C11 atomics. This eliminates mutex contention between application and transport threads, enabling safe concurrent access to the framebuffer ring. Convert `stat` in all 8 pipeline frame structs to `_Atomic uint32_t`. Stat is now acting as our lock-free state machine for the framebuffer ring, with atomic compare-and-swap (CAS) operations used to transition between states. Ensure thread-safe TX sequence number generation via atomic `__atomic_fetch_add` on `framebuff_sequence_number`. Remove all `pthread_mutex_t lock` usage, initialization, and destruction across lib/src/st2110/pipeline/. Use atomic CAS for pipeline framebuffer status transitions
Integrate upstream stress tests from commit 5fdceaa7 and add new deterministic concurrency tests for ST20, ST22, ST30, and ST40 pipelines. These tests verify the new lock-free implementation by simulating high-contention scenarios across multiple production and consumption threads. - Update pipeline harnesses to remove pthread_mutex, aligning with the library's move to lock-free buffers. - Add MultiProducerNoSpuriousClaimFailure and similar test cases to verify thread safety under stress. - Update Meson build to include the new test suites. Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
Replace GCC __atomic_* builtins with <stdatomic.h> equivalents (atomic_load_explicit / atomic_store_explicit / atomic_fetch_add_explicit / atomic_compare_exchange_*_explicit) and declare the tx framebuff sequence-number counters _Atomic. Ordering and semantics are unchanged; this aligns the pipeline with the C11 lock-free model already used by mt_handle_guard.h. Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
The lock-free framebuffer status protocol is correct only when get_frame and put_frame run on one thread and synchronously, and when transport callbacks (frame_ready, packet_convert, frame_done, query_ext_frame) run synchronously within their single owning tasklet. Record these invariants under section 6.3 so callers do not assume the get/put APIs are re-entrant or safe for concurrent multi-threaded use. Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
The TX blocking get_frame waited on block_wake_cond with a single cond_timedwait and no predicate, so a wake signalled between the failed frame claim and entering the wait was lost (the consumer then blocked the full timeout despite an available frame), and a spurious wakeup could return NULL before the timeout elapsed. Mirror the RX side: add block_wake_pending, set it in tx_*_block_wake, and loop on the predicate until a frame is signalled, the handle is destroying, or the wait times out. Signed-off-by: Dawid Wesierski <dawid.wesierski@intel.com>
Should be impossible to reach normally but test a scneario where wakeup is called spourously. Tests: 47e10a5
Sakoram
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.